Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding authentik oauth implementation #372

Open
wants to merge 11 commits into
base: trunk
Choose a base branch
from

Conversation

danejur
Copy link
Contributor

@danejur danejur commented Apr 13, 2023

Adding an OAuth authentication mechanism using Authentik as the provider. There are 5 required environment variables for this to work:

OAUTH_AUTHENTIK_CLIENT_ID
OAUTH_AUTHENTIK_CLIENT_SECRET
OAUTH_AUTHENTIK_AUTHORIZE_URL
OAUTH_AUTHENTIK_USERINFO_URL
OAUTH_AUTHENTIK_TOKEN_URL

Additionally, some setup is required in Authentik, which probably merits an entry to the documentation. I modeled all of this after the other OAuth implementations, so if something's amiss, let me know. Also, I went ahead and used a generic key icon as Tabler didn't have anything that looked even close to Authentik's iconography (from what I could tell).

I've tested it locally and can safely say that it Works on My Machine™, but I've yet to dockerize it and try it out remotely.

This PR satisfies one of the suggested additions of #321.

@github-actions github-actions bot added this to the 3.7.1 milestone Apr 13, 2023
@0xEmma
Copy link

0xEmma commented Apr 19, 2023

Built with docker(ghcr.io/0xemma/zipline:trunk), and setup with KeyCloak OAuth, only minor "bug" i notice, is that the redirect url is http, rather then the HTTPS im accessing the site via, but this might be due to my HTTPs being handled by my ingress.

@danejur
Copy link
Contributor Author

danejur commented Apr 20, 2023

@0xEmma I encountered something similar (I'm also terminating TLS at my reverse proxy), but I believe this is solved with an environment variable (see #304). Good to know that this works with other providers! Should we make this one a little more generic to handle other cases, or would it be better to add provider-specific implementations? If we make this one more generic, we could add an additional variable for provider_name.

@0xEmma
Copy link

0xEmma commented Apr 20, 2023

I think making it generic would be a good addition with the provider_name, as well as perhaps configuring the endpoints from the .openid-configuration, which returns the JSON data for the token/userinfo/authorize URL's (https://swagger.io/docs/specification/authentication/openid-connect-discovery/)

I'll see if i can add any of that to the implementation

@JustJoostNL
Copy link

Any updates on this? Would love to see an Authentik implementation! 😄

@diced
Copy link
Owner

diced commented May 10, 2023

Any updates on this? Would love to see an Authentik implementation! 😄

Have not had enough time lately to get everything setup and working. I think I'll see if i can do it by next week.

src/pages/api/user/index.ts Outdated Show resolved Hide resolved
src/pages/api/user/index.ts Outdated Show resolved Hide resolved
src/pages/api/user/index.ts Outdated Show resolved Hide resolved
src/pages/api/user/index.ts Outdated Show resolved Hide resolved
@diced
Copy link
Owner

diced commented May 11, 2023

I think this looks good, though I have not tested it with authentik yet.

@danejur Before this gets merged would you be able to make documentation on https://github.com/diced/zipline-docs?

@diced
Copy link
Owner

diced commented May 13, 2023

The buttons on the oauth selection should probably be changed in to a grid of 2x2 if all 4 are used (even if this use case is rare)
image

Other than that, looks good to merge.

@0xEmma
Copy link

0xEmma commented May 21, 2023

Not OP but made a PR for documentation diced/zipline-docs#55

@xbdmHQ
Copy link

xbdmHQ commented Jun 18, 2023

@diced Will this be merged?

@diced
Copy link
Owner

diced commented Jun 18, 2023

We are still waiting for some changes from the author, until then it won't be merged.

@danejur
Copy link
Contributor Author

danejur commented Jun 20, 2023

Hey all, just to give an update: I've been pretty slammed with a new job and real-world stuff, but fully intend to make the updates necessary here. That being said, if someone wants to take over making the required changes (I know there aren't many), I'd be happy to accept a PR into my repo to add to this PR.

@diced
Copy link
Owner

diced commented Jul 2, 2023

No problem, I am thinking of adding authentic support into V4. Since V3 will no longer receive feature updates, I'm wondering how to credit you.. Idk we'll cross that bridge when we come to it 😅, I'd likely ask you to make a small PR or something, if you don't want to do that I can just add some comments within parts you added.

diced added a commit that referenced this pull request Jul 21, 2023
@diced
Copy link
Owner

diced commented Jul 21, 2023

8b74b0b in v4 now supports authentik, im gonna keep this PR open until v4 is released though.

@CronixZero
Copy link

I'm no expert on the matter of OIDC but this does just look like a reguler OIDC authentication method. Maybe it would be great to rebrand this as a general oidc authentication method and add a "label" to be displayed instead of Authentik. I would like to check this against an identity provider that is not Authentik though

@CronixZero
Copy link

As expected, this does work with other OIDC Identity Providers. @diced I think it would be great if v4 included this as a general OIDC implementation with the option of renaming what is beeing shown (Currently Authentik).
Also, at the moment this implementation will only use the preferred_username as the key for the username. What if we made the scopes and the json key for the username customizable? That way it would be possible to have support for more Identity Providers that have different scopes. For example, the Identity Provider, I tested this with, supplies a really long name under preferred_username. By making the key customizable, it would be possible to change it to the much simpler key "name" that is just my (or the user's) name

@diced
Copy link
Owner

diced commented Aug 13, 2023

@CronixZero I unfortunately have no idea how authentik or oidc works, so i'm going off a guess here lol- i'm guessing that you changed the variables to a different service other than authentik that seems to implement the same stuff authentik does, therefore working with zipline's integration? i'd also be pretty open to changing how it is implemented to better work with other stuff after clearing this up.

@CronixZero
Copy link

CronixZero commented Aug 13, 2023

@diced Exactly, I changed Authorize, Info and TokenURL. Those are standardized by OIDC. The only thing that could change between Identity Providers are the scopes (essentialy just the information that is requested in groups)

The Authentik Authorization Flow here is essentialy just an OIDC Authorization Flow

@CronixZero
Copy link

OIDC is just the OpenID Connect standardization. Most IdentityProviders support it

@TheDevMinerTV
Copy link

TheDevMinerTV commented Aug 31, 2023

TODO:

@TheDevMinerTV
Copy link

Hey @danejur & @0xEmma!

Please have a look at the PRs in your respective repositories. I've taken some time to rename this into the proper authentication flow (OIDC).

When those PRs are merged, please update your PRs (#372 & diced/zipline-docs#55) from upstream.

@diced
Copy link
Owner

diced commented Aug 31, 2023

Hey @danejur & @0xEmma!

Please have a look at the PRs in your respective repositories. I've taken some time to rename this into the proper authentication flow (OIDC).

When those PRs are merged, please update your PRs (#372 & diced/zipline-docs#55) from upstream.

Hey! Thanks for taking the time in doing this, but I'm pretty sure you are not aware of the current circumstances. This PR won't be merged, as I have implemented this within v4 (see branch) so any changes can be done through a PR to the v4 branch. This pr is only open to serve as a discussion for Authentic/oidc support in v4 as of a month ago when it was implemented.

@TheDevMinerTV
Copy link

TheDevMinerTV commented Sep 1, 2023

This pr is only open to serve as a discussion for Authentic/oidc support in v4 as of a month ago when it was implemented.

Would you mind cherry picking my commits from their forks, or should I make new PRs?

@diced
Copy link
Owner

diced commented Sep 30, 2023

kind soul's pr #466

@diced diced removed this from the 3.7.1 milestone Nov 22, 2023
@samip5
Copy link

samip5 commented Jan 8, 2024

Do I have to be running trunk for Authentik support for the moment?

@TheCataliasTNT2k
Copy link

Do you have a plan, when this v4 version including these changes (or your own implementation) will be merged? @diced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants